Merge pull request #6551 from escattone/prevent-invalid-emails-from-spoiling-rest

only send messages to valid emails

Author: akatsoulas

kitsune/sumo/email_utils.py

@@ -4,7 +4,9 @@
 from django.conf import settings
 from django.contrib.sites.models import Site
 from django.core import mail
+from django.core.exceptions import ValidationError
 from django.core.mail import EmailMultiAlternatives
+from django.core.validators import validate_email
 from django.template.loader import render_to_string
 from django.test.client import RequestFactory
 from django.utils import translation
@@ -15,13 +17,48 @@
 log = logging.getLogger("k.email")
 
 
+def normalize_gmail(email: str) -> str:
+    """
+    Return the given email with periods removed from its "local part"
+    if it's a gmail address, otherwise return it unchanged.
+    """
+    if email.lower().endswith("@gmail.com"):
+        # Periods are ignored in the "local part" of gmail addresses.
+        # We need to remove them before using Django's "validate_email",
+        # otherwise it might incorrectly raise a ValidationError.
+        local_part, domain = email.rsplit("@", maxsplit=1)
+        return f"{local_part.replace('.', '')}@{domain}"
+    return email
+
+
+def is_valid_email(email: str) -> bool:
+    """
+    Returns True if the given email address is valid, False otherwise.
+    """
+    try:
+        validate_email(normalize_gmail(email))
+    except ValidationError:
+        return False
+    return True
+
+
 def send_messages(messages):
-    """Sends a bunch of EmailMessages."""
+    """Sends a bunch of email messages."""
     if not messages:
         return
 
+    # Only send each message to its valid recipients,
+    # excluding messages without any valid recipients.
+    cleaned_messages = []
+    for message in messages:
+        # Remove invalid emails and normalize gmails.
+        cleaned_to = [normalize_gmail(email) for email in message.to if is_valid_email(email)]
+        if cleaned_to:
+            message.to = cleaned_to
+            cleaned_messages.append(message)
+
     with mail.get_connection(fail_silently=True) as conn:
-        conn.send_messages(list(messages))
+        conn.send_messages(cleaned_messages)
 
 
 def safe_translation(f):

kitsune/sumo/tests/test_email_utils.py

@@ -2,11 +2,12 @@
 
 from django.conf import settings
 from django.contrib.sites.models import Site
+from django.core.mail import EmailMultiAlternatives
 from django.utils.functional import lazy
 from django.utils import translation
 from django.utils.translation import get_language
 
-from kitsune.sumo.email_utils import emails_with_users_and_watches, safe_translation
+from kitsune.sumo.email_utils import emails_with_users_and_watches, safe_translation, send_messages
 from kitsune.sumo.tests import TestCase
 from kitsune.users.tests import UserFactory
 
@@ -132,3 +133,26 @@ def test_styles_inlining(self):
             for m in msg:
                 tag = '<a href="https://%s/test" style="color:#000">Hyperlink</a>'
                 self.assertIn(tag % Site.objects.get_current().domain, str(m.message()))
+
+
+class SendMessagesTests(TestCase):
+
+    @patch("kitsune.sumo.email_utils.mail")
+    def test_send_messages(self, mock_mail):
+        from_email = "notifications@support.mozilla.org"
+        messages = [
+            EmailMultiAlternatives("Test", "Testing", from_email, ["beatles"]),
+            EmailMultiAlternatives(
+                "Test", "Testing", from_email, ["george.harrison.@gmail.com", "ringo"]
+            ),
+            EmailMultiAlternatives("Test", "Testing", from_email, ["paul.mccartney.@gmail.com"]),
+            EmailMultiAlternatives(
+                "Test", "Testing", from_email, ["ringo@beatles.com", "george@beatles.com"]
+            ),
+        ]
+        send_messages(messages)
+        send_messages_mock = mock_mail.get_connection().__enter__().send_messages
+        send_messages_mock.assert_called_once_with([messages[1], messages[2], messages[3]])
+        self.assertEqual(messages[1].to, ["georgeharrison@gmail.com"])
+        self.assertEqual(messages[2].to, ["paulmccartney@gmail.com"])
+        self.assertEqual(messages[3].to, ["ringo@beatles.com", "george@beatles.com"])